Skip to content

Read aloud support - #233

Merged
AbeJellinek merged 2 commits into
zotero:masterfrom
michalrentka:read-aloud
Jul 27, 2026
Merged

Read aloud support#233
AbeJellinek merged 2 commits into
zotero:masterfrom
michalrentka:read-aloud

Conversation

@michalrentka

@michalrentka michalrentka commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

I'll clear the unnecessary AI comments once this is finished and approved. Closes #215.

@michalrentka michalrentka mentioned this pull request Jul 24, 2026
Closed
@AbeJellinek

AbeJellinek commented Jul 24, 2026

Copy link
Copy Markdown
Member

Thanks! Looks totally fine to me. Definitely clean up the comments :)

AbeJellinek
AbeJellinek reviewed Jul 24, 2026
View reviewed changes
Comment thread src/index.ios.js Outdated
try {
const sdt = await window._view._loadSDT();
if (sdt) {
blockIndex = window._view._view.getVisibleBlockIndex?.(sdt.structure) ?? null;

@AbeJellinek AbeJellinek Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this drilling down into the view class belongs here. Can you add a wrapper in view.js?

@michalrentka michalrentka Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright I added a wrapper in 0e9131b

AbeJellinek added a commit that referenced this pull request Jul 24, 2026
@AbeJellinek
setReadAloudSpotlight(): Wrap selector to pass to navigate()
d40161c 
#233
AbeJellinek
AbeJellinek reviewed Jul 24, 2026
View reviewed changes
Comment thread src/index.ios.js Outdated
// the move when already visible, `block: 'center'` keeps the read text centered.
if (position) {
try {
window._view._view.navigateToSelector(position, { ifNeeded: true, block: 'center', behavior: 'smooth' });

@AbeJellinek AbeJellinek Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary after d40161c, was just a bug in view.js that needed to be fixed there.

@michalrentka michalrentka Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed 0e9131b

@AbeJellinek

AbeJellinek commented Jul 24, 2026

Copy link
Copy Markdown
Member

Noticed some things when I read more carefully on a second pass! Broken setReadAloudSpotlight() navigation was just a bug there; typical Claude obtuseness led it to implement its own fixed version of the broken logic, leaving the broken code in place, rather than actually fixing the bug. Hopefully this works.

@michalrentka

michalrentka commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Noticed some things when I read more carefully on a second pass! Broken setReadAloudSpotlight() navigation was just a bug there; typical Claude obtuseness led it to implement its own fixed version of the broken logic, leaving the broken code in place, rather than actually fixing the bug. Hopefully this works.

To be fair to claude, I specifically told it to not touch any reader internals other than index.ios.js. Last time I tried implementing epub/html read aloud support with claude, it started changing quite a lot of internal files which I had no idea about and I guess it would have just added more work to you. So that's why the code here looks a bit weirder this time.

It works fine btw.

@AbeJellinek
AbeJellinek merged commit 457b1d0 into zotero:master Jul 27, 2026
1 check passed
@AbeJellinek

AbeJellinek commented Jul 27, 2026

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AbeJellinek AbeJellinek AbeJellinek left review comments

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

EPUB / HTML: Read aloud iOS support

2 participants

@michalrentka @AbeJellinek